-
Notifications
You must be signed in to change notification settings - Fork 427
Add support for aarch64 #2973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for aarch64 #2973
Conversation
|
the nightly build ci allows adhoc trigger for a specific branch |
|
I noticed that the original issue mentioned QEMU being slow. I looked at how other projects handle this and saw that Polars recently added native ARM runners here: pola-rs/polars#25894 It looks like Github recently (good timing lol...) added |
|
Interesting! Their public docs don't mention them, so I thought QEMU was the way to go. This is absolutely perfect! I'll update this tomorrow. Bonus, we'll get to see if the test suite fails at all on ARM (not expecting it to since we have Mac CI) |
| CIBW_ARCHS: "auto64" | ||
| CIBW_ARCHS: "auto64 aarch64" | ||
| CIBW_PROJECT_REQUIRES_PYTHON: ">=3.10,<3.14" | ||
| CIBW_MUSLLINUX_X86_64_IMAGE: "musllinux_1_2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These configs are set to musllinux_1_2 by default, got a successful run without them.
|
Yeah it's super cool! The comparison helped uncover some other things with our release too. But I did some testing on my fork with these changes, and ran a nightly build with this and it worked pretty well! Nightly run: https://github.com/geruh/iceberg-python/actions/runs/21427296271 The change becomes much simpler and we can add |
|
@geruh thanks for the investigation! I was really hoping it could be that simple of a fix before I went down the QEMU rabbit hole. |
Fokko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rambleraptor for fixing this, and thanks @geruh for the thorough review. Do we feel confident to add this for the 0.11 release? I think it would be super valuable since multiple users were asking for this.
|
I'm confident since we aren't changing the CI path for any other platforms. Worst case (and I don't expect this), this would impact just the release for linux arm64 |
|
I just ran a test against the wheels in test pypi. It installed fine in Docker on both manylinux and musllinux aarch64 containers, and it seems like everything is working properly. I didn't test the full suite on ARM, but the CI already runs decoder tests on the native ARM runner and those passed. That being said I believe this is ready for 0.11.0. Now whether or not we use native runners or QEMU, the wheels should have same behavior! |
|
i ran the nightly build for this branch: https://github.com/apache/iceberg-python/actions/runs/21462770268 I see
|
|
Thanks for the PR @rambleraptor and thanks for the review @Fokko @raulcd @geruh |
Closes #2970
Rationale for this change
This should allow us to build aarch64 wheels on Linux. We have to use QEMU since GitHub doesn't have arm64 Linux runners (just Mac).
Are these changes tested?
Testing CI is awful! We shouldn't merge this in until after 0.11 and then see what happens.
Are there any user-facing changes?